Create codeql.yml #4624

gabriellavengeo · 2023-10-25T10:24:15Z

- What I did

Enable CodeQL scanning.

- How I did it

Add codeql.yml

- How to verify it

Check CodeQL Github action and code scanning results

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

codecov-commenter · 2023-10-25T10:27:35Z

Codecov Report

Merging #4624 (39b1d37) into master (b7b5b31) will not change coverage.
Report is 10 commits behind head on master.
The diff coverage is n/a.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4624   +/-   ##
=======================================
  Coverage   59.74%   59.74%           
=======================================
  Files         288      288           
  Lines       24849    24849           
=======================================
  Hits        14846    14846           
  Misses       9117     9117           
  Partials      886      886

github-advanced-security · 2023-10-25T10:28:42Z

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

thaJeztah · 2023-10-25T12:51:26Z

Thank you for contributing! It appears your commit message is missing a DCO sign-off,
causing the DCO check to fail.

We require all commit messages to have a Signed-off-by line with your name
and e-mail (see "Sign your work"
in the CONTRIBUTING.md in this repository), which looks something like:

Signed-off-by: YourFirsName YourLastName <[email protected]>

There is no need to open a new pull request, but to fix this (and make CI pass),
you need to amend the commit(s) in this pull request, and "force push" the amended
commit.

Unfortunately, it's not possible to do so through GitHub's web UI, so this needs
to be done through the git commandline.

You can find some instructions in the output of the DCO check (which can be found
in the "checks" tab on this pull request), as well as in the Moby contributing guide.

Steps to do so "roughly" come down to:

Set your name and e-mail in git's configuration:
```
git config --global user.name "YourFirstName YourLastName"
git config --global user.email "[email protected]"
```
(Make sure to use your real name (not your GitHub username/handle) and e-mail)
Clone your fork locally
Check out the branch associated with this pull request
Sign-off and amend the existing commit(s)
```
git commit --amend --no-edit --signoff
```
If your pull request contains multiple commits, either squash the commits (if
needed) or sign-off each individual commit.
Force push your branch to GitHub (using the --force or --force-with-lease flags) to update the pull request.

Sorry for the hassle (I wish GitHub would make this a bit easier to do), and let me know if you need help or more detailed instructions!

thaJeztah · 2023-10-25T12:51:45Z

Canned replies FTW 😂

thaJeztah · 2023-10-25T12:54:45Z

.github/workflows/codeql.yml

+
+on:
+  push:
+    branches: [ "master", "18.06", "18.09", "19.03", "[a-zA-Z0-9][a-zA-Z0-9].[a-zA-Z0-9]", "[a-zA-Z0-9][a-zA-Z0-9].[a-zA-Z0-9][a-zA-Z0-9]" ]


We can remove the 19.03 and older branches; they're no longer maintained (I may have locked them as well)

(FWIW I'm considering to use different naming schemes for release branches; release/XXX for actively maintained release branches, and archive/XXX for branches that are no longer maintained - assuming those branches contain possibly code that was not in the latest tag from the branch)

Canned replies FTW 😂

Lol yes

We can remove the 19.03 and older branches; they're no longer maintained (I may have locked them as well)

Sure, let's leave it just on master! We can update it if you decide to adopt the new release branch naming convention.

thaJeztah · 2023-10-26T08:07:58Z

.github/workflows/codeql.yml

+    runs-on: ${{ (matrix.language == 'swift' && 'macos-latest') || 'ubuntu-latest' }}
+    timeout-minutes: ${{ (matrix.language == 'swift' && 120) || 360 }}


Guessing this part came from a template / example; we don't have swift in this repository, which effectively means that CodeQL now only runs on ubuntu-latest.

I guess we can remove the swift condition, or make it explicitly only run on ubuntu-latest, or if running it on different platforms improves coverage of CodeQL, perhaps we should have all of Linux, macOS, and Windows

Yeah, it's from the auto-generated stuff, I'll clean it up

Sorry for being nit-picky; I expect these kind of changes to apply to other repositories where we'll be adding CodeQL as well, so probably after we tweaked things, we can copy/pasta things to the other repositories 🤗

No worries at all!

thaJeztah · 2023-10-26T08:10:33Z

.github/workflows/codeql.yml

+        # CodeQL supports [ 'c-cpp', 'csharp', 'go', 'java-kotlin', 'javascript-typescript', 'python', 'ruby', 'swift' ]
+        # Use only 'java-kotlin' to analyze code written in Java, Kotlin or both
+        # Use only 'javascript-typescript' to analyze code written in JavaScript, TypeScript or both
+        # Learn more about CodeQL language support at https://aka.ms/codeql-docs/language-support


We'll unlikely be adding any of the other (currently supported) languages, so perhaps we should remove this comment, also because that list may be out of date at any time in future.

The link to what's currently supported (https://aka.ms/codeql-docs/language-support) may still be useful though, so perhaps keep that one.

Yeah, sure, I'll clean up all the irrelevant autogenerated stuff

thaJeztah · 2023-10-26T08:15:37Z

.github/workflows/codeql.yml

+    # Autobuild attempts to build any compiled languages (C/C++, C#, Go, Java, or Swift).
+    # If this step fails, then you should remove it and run the build manually (see below)
+    - name: Autobuild
+      uses: github/codeql-action/autobuild@v2


This part feels like a lot of magic; I suspect it will just do a go build . (or along those lines), which won't match how we build our actual binaries (we may be passing specific build options).

Wondering if we can integrate this with a build-stage, and provide it access to the produced artefacts instead.

I see the lines below this outline "here's what to do if you don't want to use the auto build"

I suspect buildx and buildkit will be in a similar situation, and @crazy-max has a lot more GHA-Fu than I have; perhaps he has suggestions (and we can take a similar approach in the buildx repository - not having to reinvent the wheel)

Actually autobuild tries to detect build scripts and common build systems, so from the logs it look like it's using the Makefile (https://github.com/docker/cli/actions/runs/6641001271/job/18042547995?pr=4624#step:4:110)

As for giving it access to the produced build artefacts, that wouldn't do the trick as it does some tracing and instrumentation during compilation to do the extraction (https://docs.github.com/en/code-security/codeql-cli/codeql-cli-manual/database-create#-c---commandcommand). Same reason caching is an issue.

Hm, right. One thing I notice at least that in that case, it builds on the host (not using the Dockerfile), and will build with a different version of Go;

2023/10/25 13:19:30 Autobuilder was built with go1.21.1, environment has go1.20.10

So we may need a go-setup action added to have the same version of Go as other workflows at least.

Do we know if the CodeQL analysis can be injected into a container somehow, so that it runs in the same environment as we'd otherwise be running in? 🤔

https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/running-codeql-code-scanning-in-a-container should be doable

Hm, actually, for this to work like in the example

# Specify the container in which actions will run container: image: codeql-container:f0f91db

it would need to be an image from Hub or another configured registry, can't just give it a Dockerfile

thaJeztah · 2023-10-26T08:18:00Z

.github/workflows/codeql.yml

+
+on:
+  push:
+    branches: [ "master" ]


I guess we could use the same list as for the other workflows;

cli/.github/workflows/e2e.yml

Lines 8 to 14 in 5fc42fc

workflow_dispatch:

push:

branches:

- 'master'

- '[0-9]+.[0-9]+'

tags:

- 'v*'

Will do, thanks!

crazy-max

We already have a codeql workflow: https://github.com/docker/cli/blob/master/.github/workflows/codeql-analysis.yml

Could you just update this one and rename it codeql.yml?

@thaJeztah Yeah I got similar PR on login-action: docker/login-action#622

thaJeztah · 2023-10-26T09:33:43Z

DOH! I knew we had it, but disabled at some point, so thought we removed it 🙈

crazy-max · 2023-10-26T12:56:57Z

.github/workflows/codeql-analysis.yml

@@ -1,6 +1,15 @@
-name: codeql
+name: "CodeQL"


Same as docker/login-action#622 (comment) if you don't mind 😇

crazy-max · 2023-10-26T12:57:33Z

.github/workflows/codeql-analysis.yml

-  codeql:
-    runs-on: ubuntu-20.04
+  analyze:
+    name: Analyze


Same here docker/login-action#622 (comment)

crazy-max

LGTM!

Signed-off-by: Gabriela Georgieva <[email protected]>

crazy-max · 2023-10-26T13:25:57Z

.github/workflows/codeql.yml

+    container:
+      image: 'docker-cli-dev:latest'


Oh actually you need to remove this otherwise it fails:

Don't think we need it for codeql

Oh yeah, that wasn't supposed to go into the commit - fixed!

crazy-max

LGTM

gabriellavengeo · 2023-10-26T13:40:50Z

@crazy-max @thaJeztah Thanks for reviewing! Don't have write access to merge it myself but feel free to do so

crazy-max

I was looking at the CodeQL logs and it seems codeql go extractor does not take into account our vendor.mod: https://github.com/github/codeql/blob/dbb4167f804e4b9dc5544a4a7bc19c3e4250ac2d/go/extractor/extractor.go#L221

Which could be an issue because files in vendor are interpreted as legit source code instead of dependencies which could result in false-positives for analysis. Maybe not a real issue though as I can see: https://github.com/github/codeql/blob/dbb4167f804e4b9dc5544a4a7bc19c3e4250ac2d/go/extractor/extractor.go#L200C2-L200C14

Not a CodeQL expert though so I defer 😅

gabriellavengeo · 2023-10-26T14:22:28Z

@crazy-max yeah I think it's fine, it seems to skip them (seeing Skipping dependency package github.com/docker/cli/vendor/... in the logs)

thaJeztah

LGTM, thanks!

thaJeztah reviewed Oct 25, 2023

View reviewed changes

gabriellavengeo force-pushed the patch-1 branch from 0c4b91d to 8d99083 Compare October 25, 2023 13:18

gabriellavengeo changed the title ~~WIP:Create codeql.yml~~ Create codeql.yml Oct 25, 2023

gabriellavengeo marked this pull request as ready for review October 25, 2023 15:16

thaJeztah added status/2-code-review area/testing labels Oct 26, 2023

thaJeztah reviewed Oct 26, 2023

View reviewed changes

gabriellavengeo force-pushed the patch-1 branch from 8d99083 to 7cda83d Compare October 26, 2023 08:51

crazy-max suggested changes Oct 26, 2023

View reviewed changes

gabriellavengeo force-pushed the patch-1 branch from 7cda83d to 4f96358 Compare October 26, 2023 12:24

crazy-max reviewed Oct 26, 2023

View reviewed changes

gabriellavengeo force-pushed the patch-1 branch from 4f96358 to a0a598e Compare October 26, 2023 13:23

crazy-max approved these changes Oct 26, 2023

View reviewed changes

Update CodeQL workflow

39b1d37

Signed-off-by: Gabriela Georgieva <[email protected]>

crazy-max suggested changes Oct 26, 2023

View reviewed changes

gabriellavengeo force-pushed the patch-1 branch from a0a598e to 39b1d37 Compare October 26, 2023 13:26

crazy-max approved these changes Oct 26, 2023

View reviewed changes

crazy-max reviewed Oct 26, 2023

View reviewed changes

thaJeztah approved these changes Oct 26, 2023

View reviewed changes

thaJeztah merged commit a6351d0 into docker:master Oct 26, 2023
76 checks passed

thaJeztah added this to the 25.0.0 milestone Oct 26, 2023

thaJeztah mentioned this pull request May 6, 2024

Fix errors encountered by CodeQL #5049

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create codeql.yml #4624

Create codeql.yml #4624

gabriellavengeo commented Oct 25, 2023 •

edited

Loading

codecov-commenter commented Oct 25, 2023 •

edited

Loading

github-advanced-security bot commented Oct 25, 2023

thaJeztah commented Oct 25, 2023

thaJeztah commented Oct 25, 2023

thaJeztah Oct 25, 2023

gabriellavengeo Oct 25, 2023

thaJeztah Oct 26, 2023

gabriellavengeo Oct 26, 2023

thaJeztah Oct 26, 2023

gabriellavengeo Oct 26, 2023

thaJeztah Oct 26, 2023

gabriellavengeo Oct 26, 2023

thaJeztah Oct 26, 2023

gabriellavengeo Oct 26, 2023

thaJeztah Oct 26, 2023

gabriellavengeo Oct 26, 2023

gabriellavengeo Oct 26, 2023

thaJeztah Oct 26, 2023

gabriellavengeo Oct 26, 2023

crazy-max left a comment

thaJeztah commented Oct 26, 2023

crazy-max Oct 26, 2023

crazy-max Oct 26, 2023

crazy-max left a comment

crazy-max Oct 26, 2023

gabriellavengeo Oct 26, 2023

crazy-max left a comment

gabriellavengeo commented Oct 26, 2023

crazy-max left a comment •

edited

Loading

gabriellavengeo commented Oct 26, 2023

thaJeztah left a comment

		runs-on: ${{ (matrix.language == 'swift' && 'macos-latest') \|\| 'ubuntu-latest' }}
		timeout-minutes: ${{ (matrix.language == 'swift' && 120) \|\| 360 }}

	workflow_dispatch:
	push:
	branches:
	- 'master'
	- '[0-9]+.[0-9]+'
	tags:
	- 'v*'

Create codeql.yml #4624

Create codeql.yml #4624

Conversation

gabriellavengeo commented Oct 25, 2023 • edited Loading

codecov-commenter commented Oct 25, 2023 • edited Loading

Codecov Report

github-advanced-security bot commented Oct 25, 2023

thaJeztah commented Oct 25, 2023

thaJeztah commented Oct 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crazy-max left a comment

Choose a reason for hiding this comment

thaJeztah commented Oct 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crazy-max left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crazy-max left a comment

Choose a reason for hiding this comment

gabriellavengeo commented Oct 26, 2023

crazy-max left a comment • edited Loading

Choose a reason for hiding this comment

gabriellavengeo commented Oct 26, 2023

thaJeztah left a comment

Choose a reason for hiding this comment

gabriellavengeo commented Oct 25, 2023 •

edited

Loading

codecov-commenter commented Oct 25, 2023 •

edited

Loading

crazy-max left a comment •

edited

Loading